Make CalciteReplaceCommandIT column-order-agnostic for analytics-engine route by RyanL1997 · Pull Request #5415 · opensearch-project/sql

RyanL1997 · 2026-05-07T00:34:28Z

Description

The analytics-engine route and the v2 / Lucene path return columns in different orders when there is no explicit | fields … clause:

Parquet preserves the storage order chosen by the on-disk format
Lucene preserves _source iteration order

Both are valid given the contract verifySchema declares (set equality on column names), so positional verifyDataRows assertions over-constrain the test and fail under -Dtests.analytics.force_routing=true even when the data is semantically correct.

This change applies the same column-name-keyed match pattern @ahkcs introduced for CalcitePPLRenameIT in 59c728b (#5413), now to CalciteReplaceCommandIT.

Changes

rowOf(key1, val1, …) — private helper to build column-keyed expected rows preserving insertion order via LinkedHashMap.
verifyDataRowsByColumn(…) — looks up each cell value by column name and reorders the expected row to match the response schema before delegating to the existing positional verifyDataRows matcher.
Convert four order-sensitive tests to use the new helpers:
- testMultipleReplace
- testEmptyStringReplacement
- testMultipleFieldsInClause
- testMultiplePairsInSingleCommand
Make testReplaceNonExistentField order-agnostic on the input fields are: [...] field list — assert that the prefix and every expected field name appear in the message, but not in a fixed order.

Test results

Against the analytics-engine route with -Dtests.analytics.{force_routing,parquet_indices}=true:

Suite	Before	After
`CalciteReplaceCommandIT`	16/21 (5 column-order failures)	21/21
`CalciteNoPushdownIT > CalciteReplaceCommandIT`	16/21	21/21

The v2 path remains green.

Related Issues

OpenSearch core companion: opensearch-project/OpenSearch#21527 — wires PPL replace command + replace() / regexp_replace() functions on the analytics-engine route via DataFusion's replace / regexp_replace UDFs (Calcite-side wiring + Java-regex→Rust-regex RegexpReplaceAdapter). This SQL PR is the test-side companion: without it the core PR's CalciteReplaceCommandIT validation hits the column-order false-positive failures described above.
Pattern precedent: Default plugins.calcite.enabled=true on the unified query path #5413 — same rowOf + verifyDataRowsByColumn helper shape applied to CalcitePPLRenameIT.

Check List

New functionality includes testing.
All tests pass.
New functionality has been documented (this PR description; helper javadoc references the rename precedent).
API changes companion pull request created — N/A.
Public documentation issue/PR created — N/A.

github-actions · 2026-05-07T00:35:44Z

PR Reviewer Guide 🔍

(Review updated until commit `d853fda`)

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ Recommended focus areas for review Incomplete Error Validation The refactored `testReplaceNonExistentField` now checks for individual field names like "name", "country", "state" etc. using `verifyErrorMessageContains`. However, these are very common strings that could appear anywhere in the error message for unrelated reasons. The original test also verified system fields like `_id`, `_index`, `_score`, `_maxscore`, `_sort`, `_routing` — these are no longer checked at all. Consider whether the system fields should also be verified in an order-agnostic way. verifyErrorMessageContains(e, "field [non_existent_field] not found; input fields are:"); verifyErrorMessageContains(e, "name"); verifyErrorMessageContains(e, "country"); verifyErrorMessageContains(e, "state"); verifyErrorMessageContains(e, "month"); verifyErrorMessageContains(e, "year"); verifyErrorMessageContains(e, "age"); Row Count Mismatch The `verifyDataRowsByColumn` helper only validates that each expected row exists in the response (via `verifyDataRows`), but does not verify that the total number of rows in the response matches the number of expected rows. If the response returns extra rows, the test would still pass. Confirm that the underlying `verifyDataRows` already enforces exact row count equality. private final void verifyDataRowsByColumn( JSONObject result, Map<String, Object>... expectedRows) { JSONArray schema = result.getJSONArray("schema"); int n = schema.length(); String[] columnOrder = new String[n]; for (int i = 0; i < n; i++) { columnOrder[i] = schema.getJSONObject(i).getString("name"); } @SuppressWarnings({"unchecked", "rawtypes"}) Matcher<JSONArray>[] rowMatchers = new Matcher[expectedRows.length]; for (int r = 0; r < expectedRows.length; r++) { Object[] reordered = new Object[n]; for (int c = 0; c < n; c++) { if (!expectedRows[r].containsKey(columnOrder[c])) { throw new IllegalArgumentException( "Expected row at index " + r + " is missing canonical value for response column [" + columnOrder[c] + "]; provided keys: " + expectedRows[r].keySet()); } reordered[c] = expectedRows[r].get(columnOrder[c]); } rowMatchers[r] = rows(reordered); } verifyDataRows(result, rowMatchers); Unsafe Cast In `rowOf`, the key is cast directly with `(String) pairs[i]` without any null check or type validation. If a non-String key is accidentally passed, this will throw a `ClassCastException` with a less informative message than a dedicated validation. Consider adding an `instanceof` check with a descriptive error message. private static Map<String, Object> rowOf(Object... pairs) { if (pairs.length % 2 != 0) { throw new IllegalArgumentException("rowOf expects an even number of args (key, value, ...)"); } Map<String, Object> row = new LinkedHashMap<>(); for (int i = 0; i < pairs.length; i += 2) { row.put((String) pairs[i], pairs[i + 1]); } return row;

github-actions · 2026-05-07T00:36:07Z

PR Code Suggestions ✨

Latest suggestions up to d853fda

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Tighten field-name substring assertions in error message Checking for bare field names like `"name"`, `"age"`, etc. is too permissive — these substrings could appear anywhere in the error message (e.g., in the field `non_existent_field` itself or in surrounding text). Use more specific patterns such as `"[name,"` or `" name,"` to ensure the assertions are actually verifying the field list contents. integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteReplaceCommandIT.java [260-265] -verifyErrorMessageContains(e, "name"); -verifyErrorMessageContains(e, "country"); -verifyErrorMessageContains(e, "state"); -verifyErrorMessageContains(e, "month"); -verifyErrorMessageContains(e, "year"); +verifyErrorMessageContains(e, "[name,"); +verifyErrorMessageContains(e, "country,"); +verifyErrorMessageContains(e, "state,"); +verifyErrorMessageContains(e, "month,"); +verifyErrorMessageContains(e, "year,"); verifyErrorMessageContains(e, "age"); Suggestion importance[1-10]: 4 __ Why: The concern about bare field names like `"name"` or `"age"` matching unintended substrings is valid but low risk in this specific error message context. The improved_code uses patterns like `"[name,"` which are more specific, but the last field `"age"` still lacks a trailing delimiter, making the fix incomplete.	Low
General	Validate expected row key count matches schema column count The method does not validate that the number of columns in the response schema matches the number of keys in each expected row map. If a response returns extra or fewer columns than expected, `reordered` will be silently under/over-populated. Add a check that `n` equals the size of each expected row map. integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteReplaceCommandIT.java [557-564] private final void verifyDataRowsByColumn( JSONObject result, Map<String, Object>... expectedRows) { -JSONArray schema = result.getJSONArray("schema"); -int n = schema.length(); -String[] columnOrder = new String[n]; -for (int i = 0; i < n; i++) { - columnOrder[i] = schema.getJSONObject(i).getString("name"); -} + JSONArray schema = result.getJSONArray("schema"); + int n = schema.length(); + String[] columnOrder = new String[n]; + for (int i = 0; i < n; i++) { + columnOrder[i] = schema.getJSONObject(i).getString("name"); + } + for (int r = 0; r < expectedRows.length; r++) { + if (expectedRows[r].size() != n) { + throw new IllegalArgumentException( + "Expected row at index " + r + " has " + expectedRows[r].size() + + " keys but schema has " + n + " columns."); + } + } Suggestion importance[1-10]: 4 __ Why: Adding a size check between the expected row map and the schema column count would catch mismatches early with a clear error message. However, the existing code already throws an `IllegalArgumentException` when a key is missing via `containsKey`, so the practical impact is limited to providing a slightly earlier and more descriptive failure.	Low

Previous suggestions

Suggestions up to commit 4d9046f

Category	Suggestion	Impact
General	Validate expected row key count matches schema columns There is no validation that the number of columns in the response schema matches the number of keys in each expected row map. If a test passes an `expectedRows` map with more keys than columns in the schema, those extra keys are silently ignored, potentially masking test errors. Add a check that each expected row's key count equals `n`. integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteReplaceCommandIT.java [557-564] private final void verifyDataRowsByColumn( JSONObject result, Map<String, Object>... expectedRows) { -JSONArray schema = result.getJSONArray("schema"); -int n = schema.length(); -String[] columnOrder = new String[n]; -for (int i = 0; i < n; i++) { - columnOrder[i] = schema.getJSONObject(i).getString("name"); -} + JSONArray schema = result.getJSONArray("schema"); + int n = schema.length(); + String[] columnOrder = new String[n]; + for (int i = 0; i < n; i++) { + columnOrder[i] = schema.getJSONObject(i).getString("name"); + } + for (int r = 0; r < expectedRows.length; r++) { + if (expectedRows[r].size() != n) { + throw new IllegalArgumentException( + "Expected row at index " + r + " has " + expectedRows[r].size() + + " keys but schema has " + n + " columns"); + } + } Suggestion importance[1-10]: 5 __ Why: Adding a size check before iterating is a reasonable defensive measure that would catch mismatches between expected row keys and schema columns early. However, the existing code already throws an `IllegalArgumentException` when a key is missing from `expectedRows[r]`, so extra keys are the only undetected case, making this a minor improvement.	Low
General	Tighten field-name assertions to avoid false positives Checking for bare field names like `"name"`, `"age"`, `"state"` etc. is too broad — these substrings could appear in other parts of the error message (e.g., in the field path `non_existent_field` contains "field", and future messages could contain these words coincidentally). Consider wrapping each field name with its surrounding bracket notation (e.g., `"[name]"`) to make the assertions more precise and less prone to false positives. integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteReplaceCommandIT.java [260-265] -verifyErrorMessageContains(e, "name"); -verifyErrorMessageContains(e, "country"); -verifyErrorMessageContains(e, "state"); -verifyErrorMessageContains(e, "month"); -verifyErrorMessageContains(e, "year"); -verifyErrorMessageContains(e, "age"); +verifyErrorMessageContains(e, "[name]"); +verifyErrorMessageContains(e, "[country]"); +verifyErrorMessageContains(e, "[state]"); +verifyErrorMessageContains(e, "[month]"); +verifyErrorMessageContains(e, "[year]"); +verifyErrorMessageContains(e, "[age]"); Suggestion importance[1-10]: 4 __ Why: The suggestion to use `[name]` bracket notation is valid for reducing false positives, but the actual error message format uses comma-separated names in a list like `[name, country, ...]`, so individual bracket-wrapped names like `[name]` may not appear literally in the message. The improvement is marginal and may not work correctly with the actual error format.	Low

Suggestions up to commit 2fca52c

Category	Suggestion	Impact
General	Avoid false positives in substring field name checks Checking for bare field names like `"name"`, `"state"`, `"age"` etc. is fragile because these substrings could appear in other parts of the error message (e.g., in the field `non_existent_field` itself, or in index/routing metadata field names like `_routing`). Consider wrapping each expected field name with surrounding delimiters (e.g., `"[name]"` or `" name,"`) to ensure only actual field names are matched. integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteReplaceCommandIT.java [176-181] -verifyErrorMessageContains(e, "name"); -verifyErrorMessageContains(e, "country"); -verifyErrorMessageContains(e, "state"); -verifyErrorMessageContains(e, "month"); -verifyErrorMessageContains(e, "year"); -verifyErrorMessageContains(e, "age"); +verifyErrorMessageContains(e, "[name]"); +verifyErrorMessageContains(e, "[country]"); +verifyErrorMessageContains(e, "[state]"); +verifyErrorMessageContains(e, "[month]"); +verifyErrorMessageContains(e, "[year]"); +verifyErrorMessageContains(e, "[age]"); Suggestion importance[1-10]: 4 __ Why: The suggestion has merit since bare field names like `"name"` or `"state"` could match substrings in other parts of the error message. However, the actual error message format is unknown from the diff, so `[name]` brackets may not match the actual format either. This is a minor robustness improvement for test code.	Low
General	Improve error clarity for misaligned key-value arguments The cast `(String) pairs[i]` will throw a `ClassCastException` at runtime if a non-String key is accidentally passed (e.g., an integer value placed in the key position due to argument misalignment). Consider adding an explicit type check or using `String.valueOf(pairs[i])` to produce a clearer error message that identifies the problematic argument index. integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteReplaceCommandIT.java [421-430] private static Map<String, Object> rowOf(Object... pairs) { -if (pairs.length % 2 != 0) { - throw new IllegalArgumentException("rowOf expects an even number of args (key, value, ...)"); -} -Map<String, Object> row = new LinkedHashMap<>(); -for (int i = 0; i < pairs.length; i += 2) { - row.put((String) pairs[i], pairs[i + 1]); -} -return row; + if (pairs.length % 2 != 0) { + throw new IllegalArgumentException("rowOf expects an even number of args (key, value, ...)"); + } + Map<String, Object> row = new LinkedHashMap<>(); + for (int i = 0; i < pairs.length; i += 2) { + if (!(pairs[i] instanceof String)) { + throw new IllegalArgumentException( + "rowOf expects String keys, but argument at index " + i + " is: " + pairs[i]); + } + row.put((String) pairs[i], pairs[i + 1]); + } + return row; } Suggestion importance[1-10]: 3 __ Why: This is a defensive improvement for a test helper method `rowOf`. While the explicit type check would give a clearer error message than a `ClassCastException`, this is test utility code where misuse would be caught quickly during development, making the improvement low-impact.	Low

Suggestions up to commit e8480cb

Category	Suggestion	Impact
General	Tighten field-name substring assertions in error message Checking for bare field names like `"name"`, `"state"`, `"age"` etc. in the error message is fragile — these substrings could appear in other parts of the message (e.g., in the error prefix, field values, or index names), producing false positives. Consider wrapping each expected field name with its surrounding bracket syntax (e.g., `"[name]"`) to make the assertions more precise and less prone to accidental matches. integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteReplaceCommandIT.java [176-181] -verifyErrorMessageContains(e, "name"); -verifyErrorMessageContains(e, "country"); -verifyErrorMessageContains(e, "state"); -verifyErrorMessageContains(e, "month"); -verifyErrorMessageContains(e, "year"); -verifyErrorMessageContains(e, "age"); +verifyErrorMessageContains(e, "[name]"); +verifyErrorMessageContains(e, "[country]"); +verifyErrorMessageContains(e, "[state]"); +verifyErrorMessageContains(e, "[month]"); +verifyErrorMessageContains(e, "[year]"); +verifyErrorMessageContains(e, "[age]"); Suggestion importance[1-10]: 5 __ Why: The suggestion is valid — bare field names like `"name"` or `"state"` could match unintended parts of the error message. However, the actual error format uses a list like `[name, country, ...]` rather than `[name]` per field, so wrapping with brackets may not match the actual format either. The improvement is moderate but the proposed format may not align with the real error message structure.	Low
General	Detect duplicate keys in row builder The cast `(String) pairs[i]` will throw a `ClassCastException` at runtime if a non-String key is accidentally passed. Since the method is called with interleaved string keys and mixed-type values, a duplicate key (same column name passed twice) would silently overwrite the first value without any warning. Consider adding a duplicate-key check to catch test authoring mistakes early. integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteReplaceCommandIT.java [421-430] private static Map<String, Object> rowOf(Object... pairs) { -if (pairs.length % 2 != 0) { - throw new IllegalArgumentException("rowOf expects an even number of args (key, value, ...)"); -} -Map<String, Object> row = new LinkedHashMap<>(); -for (int i = 0; i < pairs.length; i += 2) { - row.put((String) pairs[i], pairs[i + 1]); -} -return row; + if (pairs.length % 2 != 0) { + throw new IllegalArgumentException("rowOf expects an even number of args (key, value, ...)"); + } + Map<String, Object> row = new LinkedHashMap<>(); + for (int i = 0; i < pairs.length; i += 2) { + String key = (String) pairs[i]; + if (row.containsKey(key)) { + throw new IllegalArgumentException("rowOf: duplicate key [" + key + "]"); + } + row.put(key, pairs[i + 1]); + } + return row; } Suggestion importance[1-10]: 4 __ Why: Adding a duplicate-key check to `rowOf` is a reasonable defensive measure for a test helper, preventing silent overwrites from test authoring mistakes. However, this is a test utility method and the impact is low since all call sites in the PR use distinct keys.	Low

github-actions · 2026-05-07T00:38:08Z

Persistent review updated to latest commit 2fca52c

github-actions · 2026-05-07T00:59:52Z

Persistent review updated to latest commit 4d9046f

…ne route The analytics-engine route and the v2 / Lucene path return columns in different orders when there is no explicit `| fields ...` clause: parquet preserves the storage order chosen by the on-disk format, while the Lucene path preserves `_source` iteration order. Both are valid given the contract `verifySchema` declares (set equality on column names), so positional `verifyDataRows` assertions over-constrain the test and fail under `-Dtests.analytics.force_routing=true` even when the data is correct. Apply the same column-name-keyed match pattern Kai introduced for `CalcitePPLRenameIT` in 59c728b (opensearch-project#5413): * Add `rowOf(key1, val1, ...)` to build column-keyed expected rows. * Add `verifyDataRowsByColumn(...)` to look up each cell value by column name and reorder to match the response schema before delegating to the existing positional `verifyDataRows` matcher. * Convert the four order-sensitive tests (`testMultipleReplace`, `testEmptyStringReplacement`, `testMultipleFieldsInClause`, `testMultiplePairsInSingleCommand`) to the new helpers. * Make `testReplaceNonExistentField` order-agnostic on the `input fields are: [...]` field list — assert that the prefix and every expected field name appear in the message, but not in a fixed order. Test results against analytics-engine route via `-Dtests.analytics.{force_routing,parquet_indices}=true`: 21/21 pass in both the direct `CalciteReplaceCommandIT` suite and the `CalciteNoPushdownIT > CalciteReplaceCommandIT` re-run. v2 path remains green. Companion to the OpenSearch PR onboarding PPL `replace` command + `replace()` / `regexp_replace()` functions on the analytics-engine route via DataFusion `replace` / `regexp_replace` UDFs. Signed-off-by: Jialiang Liang <jiallian@amazon.com>

github-actions · 2026-05-07T18:41:27Z

Persistent review updated to latest commit d853fda

@ignore

…ation Brings the catch-up branch up to current upstream/main (4 commits since this PR was opened) and current feature/mustang-ppl-integration (9 commits since this PR was opened), so the PR is mergeable into feature/mustang-ppl-integration without conflicts. Squashed (rather than two real merge commits) for the same DCO reason the original commit was squashed: upstream commits authored by many contributors with inconsistent or missing Signed-off-by trailers would otherwise be brought into this PR's history. Newer main commits absorbed (4): - opensearch-project#5419 (LENGTH/REGEXP_REPLACE/DATE_TRUNC unified function spec) - opensearch-project#5408 (datetime type normalization) - opensearch-project#5414 (Gradle wrapper bump + @ignore exclusion) - opensearch-project#5399 (FGAC-scoped SQL cursor continuation) Newer feature commits absorbed (9): - opensearch-project#5403 (analytics-engine optional dependency — major rewiring) - opensearch-project#5407 (Carry CalciteEvalCommandIT through helper-managed index path) - opensearch-project#5413 (Default plugins.calcite.enabled=true on unified path) - opensearch-project#5415, opensearch-project#5416, opensearch-project#5417, opensearch-project#5409, opensearch-project#5400, opensearch-project#5406 (smaller carryovers + bumps) Conflict resolutions (10 from main side, 3 from feature side): api/spec/* (LanguageSpec, UnifiedFunctionSpec, UnifiedPplSpec, UnifiedSqlSpec): took main. Main is a strict superset — adds postAnalysisRules and preCompilationRules extension points, the new FunctionSpecBuilder DSL, SCALAR category for length/regexp_replace/ date_trunc, the DatetimeExtension on PPL spec, and the CoreExtension wiring on SQL spec. PR's RELEVANCE category is preserved unchanged. api/UnifiedQueryPlanner.java, api/compiler/UnifiedQueryCompiler.java: took main. Both adopt the new postAnalysisRules / preCompilationRules hooks introduced in opensearch-project#5408 / opensearch-project#5419. core/executor/QueryService.java: composed both sides — kept HEAD's CalciteClassLoaderHelper.withCalciteClassLoader wrapper around main's StageErrorHandler stage tracking. Same pattern as the original PR resolution; both improvements are orthogonal. legacy/plugin/RestSqlAction.java: took HEAD. The 3-way merge produced a duplicated handleException/getRawErrorCode block; HEAD already contained both the delegateToV2Engine refactor and the ErrorReport unwrap from main, so HEAD is the correct superset. integ-test/build.gradle: took feature. Both sides added the same @ignore exclusion block; feature has alphabetical ordering and a more detailed comment explaining the Gradle 9.4.1 TestEventReporterAsListener cast bug. integ-test/.../CalciteEvalCommandIT.java: composed both sides. Took feature's helper-managed test_eval provisioning (createIndexByRestClient + isIndexExist guard, from opensearch-project#5407) so analytics-engine compatibility runs get a parquet-backed index. Added back PR HEAD's test_eval_agent setup (needed by the dotted-path eval tests for opensearch-project#5351) wrapped in its own isIndexExist guard for the same parquet-aware idempotency. plugin/.../TransportPPLQueryAction.java: took feature. PR opensearch-project#5403 made analytics-engine an optional dependency by moving QueryPlanExecutor from a required constructor parameter to an @Inject(optional=true) setter. Feature's design supersedes our prior wiring. plugin/.../SQLPlugin.java: took feature. The same opensearch-project#5403 simplification removed loadExtensions/EngineExtensionsHolder/executionEngineExtensions plumbing (no longer needed once analytics-engine is optionally bound). Feature retains the createSqlAnalyticsRouter method this PR introduced. plugin/.../config/EngineExtensionsHolder.java: deleted. Unreferenced after taking feature's SQLPlugin/TransportPPLQueryAction; not present on feature branch. Build: :api, :core, :opensearch-sql-plugin, :legacy compileJava + :integ-test compileTestJava all pass; unit tests pass; spotlessCheck clean. Signed-off-by: Kai Huang <ahkcs@amazon.com>

@ignore

Single squashed delivery of the long-running feature/mustang-ppl-integration branch into main, consolidating 22 feature-branch PRs plus the conflict-resolved merge of current main. Squashed because the feature branch's history includes commits with missing or mismatched Signed-off-by trailers that block DCO at this scope — the equivalent issue documented for the catch-up squashes (opensearch-project#5397). The feature branch f006e29 is retained for individual-commit lineage. ### What this delivers Analytics-engine PPL integration — a new execution path that routes Parquet-backed (non-Lucene) indices through an analytics engine while keeping Lucene-backed indices on the existing v2 / Calcite paths. Headline pieces: - Query routing (opensearch-project#5267) — PPL queries against Parquet-backed indices hand off to the analytics-engine execution path; Lucene-backed indices continue through the legacy path - Explain support (opensearch-project#5275) — EXPLAIN covers the analytics-engine path - Profiling + UnifiedQueryParser (opensearch-project#5285) — migrates PPL parsing to the unified parser and wires profiling metrics through the analytics path - extendedPlugins wiring (opensearch-project#5302) — analytics-engine attaches as an OpenSearch extension via SPI - SQL REST endpoint integration (opensearch-project#5317) — same analytics-route fork applied to the SQL transport, plus delegateToV2Engine extraction in RestSqlAction - Async QueryPlanExecutor (opensearch-project#5396) — async execution for analytics-engine plans + version bump to OpenSearch 3.7 - Optional dependency (opensearch-project#5403) — analytics-engine becomes an optional runtime dep so the SQL bundle is shippable without it - Index-setting-based routing (opensearch-project#5429) — replaces the earlier table-name-prefix heuristic with an authoritative index-setting check Supporting infrastructure: - Gradle wrapper bump to 9.4.1 (opensearch-project#5406) - Jar-hell exclusions for arrow-flight-rpc / httpcore5-h2 / httpcore5-reactive / httpclient5 (opensearch-project#5400, opensearch-project#5409) - IT plumbing: CalciteEvalCommandIT / CalciteFieldFormatCommandIT carried through the helper-managed index path (opensearch-project#5407, opensearch-project#5417); CalciteReplaceCommandIT column-order-agnostic (opensearch-project#5415); @ignore'd Calcite ITs dropped from CalciteNoPushdownIT (opensearch-project#5416) - plugins.calcite.enabled=true defaulted on the unified query path (opensearch-project#5413) - PPL_REX_MAX_MATCH_LIMIT bridged into UnifiedQueryContext (opensearch-project#5418) - Calcite tolerance fixes: array() default type (opensearch-project#5421), containsNestedAggregator flat-leaf schemas (opensearch-project#5423) - Sandbox deps switched to analytics-api JDK 21 surface (opensearch-project#5426) ### Feature-branch commits squashed (22) opensearch-project#5429, opensearch-project#5426, opensearch-project#5423, opensearch-project#5421, opensearch-project#5418, opensearch-project#5403, opensearch-project#5417, opensearch-project#5415, opensearch-project#5416, opensearch-project#5413, opensearch-project#5407, opensearch-project#5409, opensearch-project#5406, opensearch-project#5400, opensearch-project#5396, opensearch-project#5317, opensearch-project#5302, opensearch-project#5285, opensearch-project#5275, opensearch-project#5267, opensearch-project#5397, opensearch-project#5286 ### Main commits absorbed via the merge (54) Brings the branch up to current upstream/main (54 commits since the last catch-up at opensearch-project#5397, divergence point 513e1b2). Highlights: opensearch-project#5419, opensearch-project#5408, opensearch-project#5414, opensearch-project#5399, opensearch-project#5394, opensearch-project#5361, opensearch-project#5360, opensearch-project#5240, opensearch-project#5266, opensearch-project#5278, plus 44 others (bugfixes, doc updates, infra). ### Conflict resolutions (7) Resolved during the merge of main into the feature branch. Resolution kept the feature branch's analytics-engine-path semantics where main's changes would have regressed them. - api/.../UnifiedQueryContext.java Blank-line-only conflict; took main's tighter formatting. - core/.../executor/QueryService.java Kept feature's CalciteClassLoaderHelper.withCalciteClassLoader(...) wrapping (required for analytics-engine classloader isolation) and the matching import. - integ-test/build.gradle Kept feature's detailed root-cause comment on the Gradle 9.4.1 TestEventReporterAsListener workaround; kept ASCII ordering of JSONRequestIT / JoinIT and SQLFunctionsIT / ShowIT / SourceFieldIT entries. - integ-test/.../CalciteEvalCommandIT.java Kept feature's if (!TestUtils.isIndexExist(...)) idempotency guards on test_eval and test_eval_agent setup (needed for the helper-managed index analytics-engine compatibility run). - legacy/.../RestSqlAction.java Kept feature's delegateToV2Engine(...) (extracted from the analytics-engine routing path). Both sides added handleException / getRestStatus / getRawErrorCode; removed the duplicate set git produced. - plugin/.../SQLPlugin.java Took the union of imports: ExecutionEngine + ExecutionEngine.ExplainResponse + QueryType. - plugin/.../transport/TransportPPLQueryAction.java Combined main's OpenSearchPluginModule(extensionsHolder.engines()) and feature's local pluginSettings / pluginSettingsRef wiring. EngineExtensionsHolder.java is a new file from main (opensearch-project#5298) preserved as-is. ### Compatibility / opt-in The analytics-engine path is gated by the extendedPlugins extension being installed (opensearch-project#5403 makes the dep optional). Clusters without analytics-engine installed see no behavior change. Clusters with analytics-engine installed route only Parquet-backed indices through the new path (opensearch-project#5429 — by index setting). ### Verification - ./gradlew :api:compileJava :core:compileJava :legacy:compileJava :opensearch-sql-plugin:compileJava :integ-test:compileTestJava passes locally Signed-off-by: Kai Huang <ahkcs@amazon.com>

@ignore

Single squashed delivery of the long-running feature/mustang-ppl-integration branch into main, consolidating 22 feature-branch PRs plus the conflict-resolved merge of current main. Squashed because the feature branch's history includes commits with missing or mismatched Signed-off-by trailers that block DCO at this scope — the equivalent issue documented for the catch-up squashes (opensearch-project#5397). The feature branch f006e29 is retained for individual-commit lineage. ### What this delivers Analytics-engine PPL integration — a new execution path that routes Parquet-backed (non-Lucene) indices through an analytics engine while keeping Lucene-backed indices on the existing v2 / Calcite paths. Headline pieces: - Query routing (opensearch-project#5267) — PPL queries against Parquet-backed indices hand off to the analytics-engine execution path; Lucene-backed indices continue through the legacy path - Explain support (opensearch-project#5275) — EXPLAIN covers the analytics-engine path - Profiling + UnifiedQueryParser (opensearch-project#5285) — migrates PPL parsing to the unified parser and wires profiling metrics through the analytics path - extendedPlugins wiring (opensearch-project#5302) — analytics-engine attaches as an OpenSearch extension via SPI - SQL REST endpoint integration (opensearch-project#5317) — same analytics-route fork applied to the SQL transport, plus delegateToV2Engine extraction in RestSqlAction - Async QueryPlanExecutor (opensearch-project#5396) — async execution for analytics-engine plans + version bump to OpenSearch 3.7 - Optional dependency (opensearch-project#5403) — analytics-engine becomes an optional runtime dep so the SQL bundle is shippable without it - Index-setting-based routing (opensearch-project#5429, opensearch-project#5432) — replaces the earlier table-name-prefix heuristic with an authoritative index-setting check Supporting infrastructure: - Gradle wrapper bump to 9.4.1 (opensearch-project#5406) - Jar-hell exclusions for arrow-flight-rpc / httpcore5-h2 / httpcore5-reactive / httpclient5 (opensearch-project#5400, opensearch-project#5409) - IT plumbing: CalciteEvalCommandIT / CalciteFieldFormatCommandIT carried through the helper-managed index path (opensearch-project#5407, opensearch-project#5417); CalciteReplaceCommandIT column-order-agnostic (opensearch-project#5415); @ignore'd Calcite ITs dropped from CalciteNoPushdownIT (opensearch-project#5416) - plugins.calcite.enabled=true defaulted on the unified query path (opensearch-project#5413) - PPL_REX_MAX_MATCH_LIMIT bridged into UnifiedQueryContext (opensearch-project#5418) - Calcite tolerance fixes: array() default type (opensearch-project#5421), containsNestedAggregator flat-leaf schemas (opensearch-project#5423) - Sandbox deps switched to analytics-api JDK 21 surface (opensearch-project#5426) ### Feature-branch commits squashed (22) opensearch-project#5432, opensearch-project#5429, opensearch-project#5426, opensearch-project#5423, opensearch-project#5421, opensearch-project#5418, opensearch-project#5403, opensearch-project#5417, opensearch-project#5415, opensearch-project#5416, opensearch-project#5413, opensearch-project#5407, opensearch-project#5409, opensearch-project#5406, opensearch-project#5400, opensearch-project#5396, opensearch-project#5317, opensearch-project#5302, opensearch-project#5285, opensearch-project#5275, opensearch-project#5267, opensearch-project#5397, opensearch-project#5286 ### Main commits absorbed via the merge (54) Brings the branch up to current upstream/main (54 commits since the last catch-up at opensearch-project#5397, divergence point 513e1b2). Highlights: opensearch-project#5419, opensearch-project#5408, opensearch-project#5414, opensearch-project#5399, opensearch-project#5394, opensearch-project#5361, opensearch-project#5360, opensearch-project#5240, opensearch-project#5266, opensearch-project#5278, plus 44 others (bugfixes, doc updates, infra). ### Conflict resolutions (7) Resolved during the merge of main into the feature branch. Resolution kept the feature branch's analytics-engine-path semantics where main's changes would have regressed them. - api/.../UnifiedQueryContext.java Blank-line-only conflict; took main's tighter formatting. - core/.../executor/QueryService.java Kept feature's CalciteClassLoaderHelper.withCalciteClassLoader(...) wrapping (required for analytics-engine classloader isolation) and the matching import. - integ-test/build.gradle Kept feature's detailed root-cause comment on the Gradle 9.4.1 TestEventReporterAsListener workaround; kept ASCII ordering of JSONRequestIT / JoinIT and SQLFunctionsIT / ShowIT / SourceFieldIT entries. - integ-test/.../CalciteEvalCommandIT.java Kept feature's if (!TestUtils.isIndexExist(...)) idempotency guards on test_eval and test_eval_agent setup (needed for the helper-managed index analytics-engine compatibility run). - legacy/.../RestSqlAction.java Kept feature's delegateToV2Engine(...) (extracted from the analytics-engine routing path). Both sides added handleException / getRestStatus / getRawErrorCode; removed the duplicate set git produced. - plugin/.../SQLPlugin.java Took the union of imports: ExecutionEngine + ExecutionEngine.ExplainResponse + QueryType. - plugin/.../transport/TransportPPLQueryAction.java Combined main's OpenSearchPluginModule(extensionsHolder.engines()) and feature's local pluginSettings / pluginSettingsRef wiring. EngineExtensionsHolder.java is a new file from main (opensearch-project#5298) preserved as-is. ### Compatibility / opt-in The analytics-engine path is gated by the extendedPlugins extension being installed (opensearch-project#5403 makes the dep optional). Clusters without analytics-engine installed see no behavior change. Clusters with analytics-engine installed route only Parquet-backed indices through the new path (opensearch-project#5429 — by index setting). ### Verification - ./gradlew :api:compileJava :core:compileJava :legacy:compileJava :opensearch-sql-plugin:compileJava :integ-test:compileTestJava passes locally Signed-off-by: Kai Huang <ahkcs@amazon.com> Co-authored-by: bowenlan-amzn <bowenlan23@gmail.com>

@ignore

* Land analytics-engine PPL integration into main Single squashed delivery of the long-running feature/mustang-ppl-integration branch into main, consolidating 22 feature-branch PRs plus the conflict-resolved merge of current main. Squashed because the feature branch's history includes commits with missing or mismatched Signed-off-by trailers that block DCO at this scope — the equivalent issue documented for the catch-up squashes (#5397). The feature branch f006e29 is retained for individual-commit lineage. ### What this delivers Analytics-engine PPL integration — a new execution path that routes Parquet-backed (non-Lucene) indices through an analytics engine while keeping Lucene-backed indices on the existing v2 / Calcite paths. Headline pieces: - Query routing (#5267) — PPL queries against Parquet-backed indices hand off to the analytics-engine execution path; Lucene-backed indices continue through the legacy path - Explain support (#5275) — EXPLAIN covers the analytics-engine path - Profiling + UnifiedQueryParser (#5285) — migrates PPL parsing to the unified parser and wires profiling metrics through the analytics path - extendedPlugins wiring (#5302) — analytics-engine attaches as an OpenSearch extension via SPI - SQL REST endpoint integration (#5317) — same analytics-route fork applied to the SQL transport, plus delegateToV2Engine extraction in RestSqlAction - Async QueryPlanExecutor (#5396) — async execution for analytics-engine plans + version bump to OpenSearch 3.7 - Optional dependency (#5403) — analytics-engine becomes an optional runtime dep so the SQL bundle is shippable without it - Index-setting-based routing (#5429, #5432) — replaces the earlier table-name-prefix heuristic with an authoritative index-setting check Supporting infrastructure: - Gradle wrapper bump to 9.4.1 (#5406) - Jar-hell exclusions for arrow-flight-rpc / httpcore5-h2 / httpcore5-reactive / httpclient5 (#5400, #5409) - IT plumbing: CalciteEvalCommandIT / CalciteFieldFormatCommandIT carried through the helper-managed index path (#5407, #5417); CalciteReplaceCommandIT column-order-agnostic (#5415); @ignore'd Calcite ITs dropped from CalciteNoPushdownIT (#5416) - plugins.calcite.enabled=true defaulted on the unified query path (#5413) - PPL_REX_MAX_MATCH_LIMIT bridged into UnifiedQueryContext (#5418) - Calcite tolerance fixes: array() default type (#5421), containsNestedAggregator flat-leaf schemas (#5423) - Sandbox deps switched to analytics-api JDK 21 surface (#5426) ### Feature-branch commits squashed (22) #5432, #5429, #5426, #5423, #5421, #5418, #5403, #5417, #5415, #5416, #5413, #5407, #5409, #5406, #5400, #5396, #5317, #5302, #5285, #5275, #5267, #5397, #5286 ### Main commits absorbed via the merge (54) Brings the branch up to current upstream/main (54 commits since the last catch-up at #5397, divergence point 513e1b2). Highlights: #5419, #5408, #5414, #5399, #5394, #5361, #5360, #5240, #5266, #5278, plus 44 others (bugfixes, doc updates, infra). ### Conflict resolutions (7) Resolved during the merge of main into the feature branch. Resolution kept the feature branch's analytics-engine-path semantics where main's changes would have regressed them. - api/.../UnifiedQueryContext.java Blank-line-only conflict; took main's tighter formatting. - core/.../executor/QueryService.java Kept feature's CalciteClassLoaderHelper.withCalciteClassLoader(...) wrapping (required for analytics-engine classloader isolation) and the matching import. - integ-test/build.gradle Kept feature's detailed root-cause comment on the Gradle 9.4.1 TestEventReporterAsListener workaround; kept ASCII ordering of JSONRequestIT / JoinIT and SQLFunctionsIT / ShowIT / SourceFieldIT entries. - integ-test/.../CalciteEvalCommandIT.java Kept feature's if (!TestUtils.isIndexExist(...)) idempotency guards on test_eval and test_eval_agent setup (needed for the helper-managed index analytics-engine compatibility run). - legacy/.../RestSqlAction.java Kept feature's delegateToV2Engine(...) (extracted from the analytics-engine routing path). Both sides added handleException / getRestStatus / getRawErrorCode; removed the duplicate set git produced. - plugin/.../SQLPlugin.java Took the union of imports: ExecutionEngine + ExecutionEngine.ExplainResponse + QueryType. - plugin/.../transport/TransportPPLQueryAction.java Combined main's OpenSearchPluginModule(extensionsHolder.engines()) and feature's local pluginSettings / pluginSettingsRef wiring. EngineExtensionsHolder.java is a new file from main (#5298) preserved as-is. ### Compatibility / opt-in The analytics-engine path is gated by the extendedPlugins extension being installed (#5403 makes the dep optional). Clusters without analytics-engine installed see no behavior change. Clusters with analytics-engine installed route only Parquet-backed indices through the new path (#5429 — by index setting). ### Verification - ./gradlew :api:compileJava :core:compileJava :legacy:compileJava :opensearch-sql-plugin:compileJava :integ-test:compileTestJava passes locally Signed-off-by: Kai Huang <ahkcs@amazon.com> Co-authored-by: bowenlan-amzn <bowenlan23@gmail.com> * Address @penghuo: revert stray blank line in doctest/build.gradle After 'apply plugin: opensearch.testclusters', one blank line is enough — restoring the single-blank spacing to match upstream/main. Signed-off-by: Kai Huang <ahkcs@amazon.com> --------- Signed-off-by: Kai Huang <ahkcs@amazon.com> Co-authored-by: bowenlan-amzn <bowenlan23@gmail.com>

RyanL1997 requested review from LantaoJin, Swiddis, acarbonetto, ahkcs, anirudha, dai-chen, joshuali925, mengweieric, noCharger, penghuo, ps48, qianheng-aws, songkant-aws, vamsimanohar, ykmr1224 and yuancu as code owners May 7, 2026 00:34

RyanL1997 force-pushed the mustang-replace-it-order-agnostic branch from e8480cb to 2fca52c Compare May 7, 2026 00:36

RyanL1997 added testing Related to improving software testing PPL Piped processing language labels May 7, 2026

ahkcs approved these changes May 7, 2026

View reviewed changes

RyanL1997 mentioned this pull request May 7, 2026

[Analytics Backend / DataFusion] Wire PPL replace command + replace()/regexp_replace() functions opensearch-project/OpenSearch#21527

Merged

5 tasks

RyanL1997 force-pushed the mustang-replace-it-order-agnostic branch from 2fca52c to 4d9046f Compare May 7, 2026 00:58

RyanL1997 force-pushed the mustang-replace-it-order-agnostic branch from 4d9046f to d853fda Compare May 7, 2026 18:40

ahkcs merged commit 5d31506 into opensearch-project:feature/mustang-ppl-integration May 7, 2026
34 of 38 checks passed

This was referenced May 11, 2026

Land analytics-engine PPL integration into main #5431

Closed

Land analytics-engine PPL integration into main #5430

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make CalciteReplaceCommandIT column-order-agnostic for analytics-engine route#5415

Make CalciteReplaceCommandIT column-order-agnostic for analytics-engine route#5415
ahkcs merged 1 commit into
opensearch-project:feature/mustang-ppl-integrationfrom
RyanL1997:mustang-replace-it-order-agnostic

RyanL1997 commented May 7, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 7, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 7, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 7, 2026

Uh oh!

github-actions Bot commented May 7, 2026

Uh oh!

github-actions Bot commented May 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

RyanL1997 commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Test results

Related Issues

Check List

Uh oh!

github-actions Bot commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Reviewer Guide 🔍

(Review updated until commit d853fda)

Uh oh!

github-actions Bot commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Previous suggestions

Uh oh!

github-actions Bot commented May 7, 2026

Uh oh!

github-actions Bot commented May 7, 2026

Uh oh!

github-actions Bot commented May 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

RyanL1997 commented May 7, 2026 •

edited

Loading

github-actions Bot commented May 7, 2026 •

edited

Loading

(Review updated until commit `d853fda`)

github-actions Bot commented May 7, 2026 •

edited

Loading